BTreeMap: disentangle Drop implementation from IntoIter #81486

ssomers · 2021-01-28T18:53:02Z

No longer require every BTreeMap to dig up its last leaf edge before dying. This speeds up the clone_ benchmarks by 25% for normal keys and values (far less for huge values).

r? @Mark-Simulacrum

library/alloc/src/collections/btree/map.rs

ssomers · 2021-01-29T12:37:38Z

Last commit conforms to #81073, dismisses two ManualDrop, introduces finalize.

library/alloc/src/collections/btree/map.rs

ssomers · 2021-01-31T15:26:22Z

I wasn't happy with the rather primitive code in finalize that belongs in navigate.rs, so ended up slightly rearranging the deallocating code. Now the benchmarks are more spectacular, though not for huge values:

benchcmp old new --threshold 10
 name                                           old ns/iter  new ns/iter  diff ns/iter   diff %  speedup
 btree::map::clone_fat_100                      45,646       72,143             26,497   58.05%   x 0.63
 btree::map::clone_fat_100_and_clear            45,567       81,445             35,878   78.74%   x 0.56
 btree::map::clone_fat_100_and_drain_half       106,640      124,207            17,567   16.47%   x 0.86
 btree::map::clone_fat_val_100                  23,640       38,426             14,786   62.55%   x 0.62
 btree::map::clone_fat_val_100_and_clear        23,687       41,178             17,491   73.84%   x 0.58
 btree::map::clone_fat_val_100_and_drain_half   48,727       57,688              8,961   18.39%   x 0.84
 btree::map::clone_fat_val_100_and_into_iter    34,402       41,846              7,444   21.64%   x 0.82
 btree::map::clone_fat_val_100_and_remove_half  52,027       61,435              9,408   18.08%   x 0.85
 btree::map::clone_slim_100                     2,184        1,696                -488  -22.34%   x 1.29
 btree::map::clone_slim_100_and_clear           5,088        1,670              -3,418  -67.18%   x 3.05
 btree::map::clone_slim_100_and_drain_all       6,518        3,560              -2,958  -45.38%   x 1.83
 btree::map::clone_slim_100_and_drain_half      5,963        2,722              -3,241  -54.35%   x 2.19
 btree::map::clone_slim_100_and_into_iter       5,100        1,655              -3,445  -67.55%   x 3.08
 btree::map::clone_slim_100_and_pop_all         6,765        3,803              -2,962  -43.78%   x 1.78
 btree::map::clone_slim_100_and_remove_all      7,826        4,944              -2,882  -36.83%   x 1.58
 btree::map::clone_slim_100_and_remove_half     6,131        3,031              -3,100  -50.56%   x 2.02
 btree::map::clone_slim_10k                     262,765      201,860           -60,905  -23.18%   x 1.30
 btree::map::clone_slim_10k_and_clear           262,713      204,480           -58,233  -22.17%   x 1.28
 btree::map::clone_slim_10k_and_into_iter       261,446      206,657           -54,789  -20.96%   x 1.27
 btree::map::clone_slim_10k_and_remove_half     574,250      512,200           -62,050  -10.81%   x 1.12
 btree::map::find_seq_100                       15           12                     -3  -20.00%   x 1.25
 btree::map::first_and_last_100                 47           42                     -5  -10.64%   x 1.12
 btree::set::clone_100                          1,883        1,352                -531  -28.20%   x 1.39
 btree::set::clone_100_and_clear                1,920        1,384                -536  -27.92%   x 1.39
 btree::set::clone_100_and_into_iter            1,928        1,410                -518  -26.87%   x 1.37
 btree::set::clone_100_and_remove_half          2,685        2,248                -437  -16.28%   x 1.19
 btree::set::clone_10k                          225,550      157,252           -68,298  -30.28%   x 1.43
 btree::set::clone_10k_and_clear                224,802      165,220           -59,582  -26.50%   x 1.36
 btree::set::clone_10k_and_into_iter            224,367      167,968           -56,399  -25.14%   x 1.34
 btree::set::clone_10k_and_remove_half          501,320      447,430           -53,890  -10.75%   x 1.12
 btree::set::difference_random_100_vs_10k       2,261        2,830                 569   25.17%   x 0.80
 btree::set::difference_staggered_100_vs_10k    2,804        2,150                -654  -23.32%   x 1.30
 btree::set::intersection_random_100_vs_10k     2,136        2,650                 514   24.06%   x 0.81
 btree::set::intersection_random_10k_vs_100     2,163        2,576                 413   19.09%   x 0.84
 btree::set::intersection_staggered_100_vs_10k  2,597        1,980                -617  -23.76%   x 1.31
 btree::set::is_subset_100_vs_10k               1,619        1,182                -437  -26.99%   x 1.37

bors · 2021-02-07T16:49:57Z

☔ The latest upstream changes (presumably #81853) made this pull request unmergeable. Please resolve the merge conflicts.

Mark-Simulacrum · 2021-02-09T00:03:02Z

@bors try @rust-timer queue

rust-timer · 2021-02-09T00:03:03Z

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

bors · 2021-02-09T00:03:12Z

⌛ Trying commit 4230d999ee1d76ab09380e172dc418f9f925be6b with merge c3d1be590b1ba9ceb83fe3737f5cf2e767f72377...

bors · 2021-02-09T00:44:12Z

☀️ Try build successful - checks-actions
Build commit: c3d1be590b1ba9ceb83fe3737f5cf2e767f72377 (c3d1be590b1ba9ceb83fe3737f5cf2e767f72377)

rust-timer · 2021-02-09T00:44:13Z

Queued c3d1be590b1ba9ceb83fe3737f5cf2e767f72377 with parent 0fc6756, future comparison URL.

rust-timer · 2021-02-09T05:36:09Z

Finished benchmarking try commit (c3d1be590b1ba9ceb83fe3737f5cf2e767f72377): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf

ssomers · 2021-02-09T12:55:48Z

Rebased and tweaked a few comments.

Mark-Simulacrum · 2021-02-12T01:33:35Z

@bors r+

Performance seems largely "noise" I guess, hard to say precisely. Change seems good from sort of "first principles" and I like that it removes the macros, at least partially; that seems like a win.

bors · 2021-02-12T01:33:37Z

📌 Commit 3045b75 has been approved by Mark-Simulacrum

bors · 2021-02-12T06:34:28Z

⌛ Testing commit 3045b75 with merge a118ee2...

bors · 2021-02-12T09:28:15Z

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing a118ee2 to master...

…lang#86031

…r=Mark-Simulacrum BTree: merge the complication introduced by rust-lang#81486 and rust-lang#86031 Also: - Deallocate the last few tree nodes as soon as an `into_iter` iterator steps beyond the end, instead of waiting around for the drop of the iterator (just to share more code). - Symmetric code for backward iteration. - Mark unsafe the methods on dying handles, modelling dying handles after raw pointers: it's the caller's responsibility to use them safely. r? `@Mark-Simulacrum`

rust-highfive assigned Mark-Simulacrum Jan 28, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 28, 2021

cuviper reviewed Jan 28, 2021

View reviewed changes

library/alloc/src/collections/btree/map.rs Outdated Show resolved Hide resolved

ssomers force-pushed the btree_separate_drop branch from bd0f6de to 0faf35a Compare January 29, 2021 12:32

cuviper reviewed Jan 29, 2021

View reviewed changes

library/alloc/src/collections/btree/map.rs Outdated Show resolved Hide resolved

ssomers force-pushed the btree_separate_drop branch 2 times, most recently from bd5b605 to b6012c3 Compare January 31, 2021 15:21

ssomers force-pushed the btree_separate_drop branch from b6012c3 to fd9918e Compare February 4, 2021 18:06

ssomers mentioned this pull request Feb 5, 2021

BTreeMap: clean up implementation of clone #81276

Closed

ssomers force-pushed the btree_separate_drop branch from fd9918e to 4230d99 Compare February 7, 2021 19:07

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Feb 9, 2021

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Feb 9, 2021

BTreeMap: disentangle Drop implementation from IntoIter

3045b75

ssomers force-pushed the btree_separate_drop branch from 4230d99 to 3045b75 Compare February 9, 2021 12:55

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 12, 2021

bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 12, 2021

bors merged commit a118ee2 into rust-lang:master Feb 12, 2021

rustbot added this to the 1.52.0 milestone Feb 12, 2021

bors mentioned this pull request Feb 12, 2021

BTree: move more shared iterator code into navigate.rs #81937

Merged

ssomers deleted the btree_separate_drop branch February 12, 2021 10:15

ssomers mentioned this pull request Aug 1, 2021

BTree: lazily locate leaves in rangeless iterators #86031

Merged

ssomers added a commit to ssomers/rust that referenced this pull request Aug 2, 2021

BTree: merge the complication introduced by rust-lang#81486 and rust-…

7b28036

…lang#86031

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BTreeMap: disentangle Drop implementation from IntoIter #81486

BTreeMap: disentangle Drop implementation from IntoIter #81486

ssomers commented Jan 28, 2021

ssomers commented Jan 29, 2021

ssomers commented Jan 31, 2021 •

edited

Loading

bors commented Feb 7, 2021

Mark-Simulacrum commented Feb 9, 2021

rust-timer commented Feb 9, 2021

bors commented Feb 9, 2021

bors commented Feb 9, 2021

rust-timer commented Feb 9, 2021

rust-timer commented Feb 9, 2021

ssomers commented Feb 9, 2021

Mark-Simulacrum commented Feb 12, 2021

bors commented Feb 12, 2021

bors commented Feb 12, 2021

bors commented Feb 12, 2021

BTreeMap: disentangle Drop implementation from IntoIter #81486

BTreeMap: disentangle Drop implementation from IntoIter #81486

Conversation

ssomers commented Jan 28, 2021

ssomers commented Jan 29, 2021

ssomers commented Jan 31, 2021 • edited Loading

bors commented Feb 7, 2021

Mark-Simulacrum commented Feb 9, 2021

rust-timer commented Feb 9, 2021

bors commented Feb 9, 2021

bors commented Feb 9, 2021

rust-timer commented Feb 9, 2021

rust-timer commented Feb 9, 2021

ssomers commented Feb 9, 2021

Mark-Simulacrum commented Feb 12, 2021

bors commented Feb 12, 2021

bors commented Feb 12, 2021

bors commented Feb 12, 2021

ssomers commented Jan 31, 2021 •

edited

Loading